feat(creator-keys): expose subscription access gating with on-chain hold check - #988
Merged
Chucks1093 merged 5 commits intoSep 27, 2026
Merged
Conversation
…old check Closes accesslayerorg#953 curve_subscriptions_swaps.rs already held subscribe_key_access and is_subscribed, but the module had no `mod` declaration, so neither compiled into the crate or could be called. Registered it. The helpers take the subscriber's balance and the minimum as parameters. That is fine internally and must never be the contract's surface: a caller-supplied subscriber_balance makes the minimum-hold check self-attested, so any wallet could claim to hold enough and gate itself in. The new entry points read both from storage instead — balance via get_key_balance, minimum from DataKey::MinHoldForAccess — so the threshold is enforced against what the ledger says. Adds set_min_hold_for_access (admin only), get_min_hold_for_access, subscribe, is_subscribed and get_subscription. The creator cannot set their own threshold: it gates paid access, and a creator who could lower it at will could admit wallets holding nothing, which is what the gate exists to prevent. Automatic revocation is evaluated on read rather than swept by a job, so there is no window in which a sold-down wallet still passes. A zero minimum is rejected because it would gate nothing while looking configured, and an unconfigured creator is reported distinctly from an insufficient balance so an operator can tell the two apart. 13 tests drive it through the contract entry points, buying and selling real keys to move balances rather than passing a balance in.
|
@ayinde38 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Fixes the verify job's format check. curve_subscriptions_swaps.rs is included because registering the module brought a file that had never been compiled — and so had never been formatted — into cargo fmt's scope.
…ix auth Restores 16 DataKey variants (StakeNft*, Vault*, FeeTiers, StakedKeys, CreatorCurveSlope and others) that the earlier "Merge branch 'main'" commit dropped from the enum while keeping their usages, which is why clippy reported a wall of E0599s against code this PR never touched. The enum is now upstream's plus MinHoldForAccess. Registering curve_subscriptions_swaps brought a file that had never been compiled into clippy's scope, surfacing three pre-existing problems in it: a crate-root-only `#![no_std]`, an unused `Vec` import, and an eight-argument `execute_atomic_swap`. The first two are removed; the third is allowed narrowly rather than refactored, since reshaping another feature's public signature is not this change's business. subscribe() no longer calls require_auth: subscribe_key_access already does, and a second call on the same frame fails with Auth(ExistingValue). This was failing 6 of the 13 tests. Authorization is still enforced before any state change, and the cheap argument checks now run before a signature is demanded. Test-side: the client type comes from the crate, not the test helper module, and a duplicate Ledger import is gone. Verified locally: cargo fmt --all --check clean, cargo clippy --workspace --all-targets -- -D warnings clean, cargo test --workspace passes with all 13 new tests green.
Running `cargo test --workspace` locally regenerated 551 Soroban test_snapshots/*.json artifacts, and the previous commit staged them — 554 files and 138k insertions of noise around a three-file change. Every snapshot is restored to its upstream state and the three newly generated ones are removed, so this branch's diff against main is now exactly the three files it should be. Reverted in a follow-up rather than amended because the bad commit had already been pushed.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #953
Closes #899
Closes #959
Closes #977
Two problems, not one
The module was never compiled in.
creator-keys/src/curve_subscriptions_swaps.rsalready containedsubscribe_key_access,is_subscribed,KeySubscriptionandSUBSCRIPTION_GRANTED_EVENT, butlib.rshas nomod curve_subscriptions_swaps;. None of it existed in the built contract. Now declared.The helper's signature can't be the contract's surface. It takes the balance and the minimum as parameters:
Exposed as-is, the minimum-hold check is self-attested — any wallet passes
subscriber_balance: u32::MAXand gates itself in. Fine for an internal helper; a hole as an entry point.The new entry points read both values from storage: balance via
get_key_balance, minimum fromDataKey::MinHoldForAccess. The threshold is enforced against what the ledger actually says.Added
set_min_hold_for_access(admin, creator, min_keys)get_min_hold_for_access(creator)Nonewhen gating is offsubscribe(creator, subscriber, duration_ledgers)is_subscribed(creator, subscriber)get_subscription(creator, subscriber)Decisions worth review
The creator cannot set their own threshold — admin only. This gates paid access. A creator who could lower it at will could admit wallets holding nothing, which is exactly what the gate exists to prevent. Say the word if you'd rather creators self-serve.
Automatic revocation is evaluated on read, not swept by a job.
is_subscribedre-checks the live balance against the minimum recorded on the subscription, so access lapses the moment a holder sells below the threshold — no revocation transaction, and no window in which a sold-down wallet still passes.Dropping below the minimum doesn't destroy the record, so topping back up restores access for the remainder of the term. That felt right for a paid subscription, but it is a product call — there's a test either way, so flipping it is a one-line change.
Raising the minimum doesn't retroactively revoke. The subscription stores the minimum in force when granted, so an increase applies to new subscriptions rather than voiding paid-for access mid-term.
A zero minimum is rejected — it would gate nothing while looking configured. Removing the key is the honest way to disable gating, and an unconfigured creator returns
NotRegisteredrather thanInsufficientBalanceso an operator can tell "gating is off" from "you need more keys".get_subscriptionis exposed because the boolean alone can't tell a caller why access was denied — expired, or under threshold — and a UI needs to say which.Verification
Not run. 13 tests in
creator-keys/tests/subscription_access_gating.rs, written against the repo's existingcontract_test_envhelpers and driving everything through the contract entry points (buying and selling real keys to move balances rather than passing a balance in). They have never been compiled or executed — please treat as unverified.The most likely thing to need a fix: my
setup()helper leaks theEnvviaBox::leakto get a'staticclient out of a tuple-returning function. It matches what the surrounding tests achieve with an inlinelet env = ..., but if the crate's test style objects, inlining setup per test removes the need for it.